-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ATARI environments - part1 #277
Conversation
…i (NON-vectorized env only)
for more information, see https://pre-commit.ci
restart test pipeline...
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…nto xfail_tests_mac_windows
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one note: I need help understanding what the ScalarizeEnvWrapper is doing here. It feels like it really only should be used when n_envs = 1
; otherwise, you're giving the same action to potentially different environments, right?
Otherwise, LGTM.
rlberry/agents/torch/utils/models.py
Outdated
def convolutions(self, x): | ||
x = x.float() | ||
if len(x.shape) == 3: | ||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe move this comment to the line above so black doesn't split this into three lines
rlberry/envs/gym_make.py
Outdated
scalarize = True | ||
|
||
if "atari_wrappers_dict" in kwargs.keys(): | ||
atari_wrappers_dict = kwargs["atari_wrappers_dict"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just do atari_wrappers_dict = kwargs.pop('atari_wrappers_dict')
for the same effect.
rlberry/envs/gym_make.py
Outdated
@@ -32,14 +33,59 @@ def gym_make(id, wrap_spaces=False, **kwargs): | |||
return Wrapper(env, wrap_spaces=wrap_spaces) | |||
|
|||
|
|||
def atari_make(id, scalarize=True, **kwargs): | |||
def atari_make(id, scalarize=None, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why True
to None
? Shouldn't it be False
?
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small comment, otherwise LGTM.
@@ -32,14 +33,58 @@ def gym_make(id, wrap_spaces=False, **kwargs): | |||
return Wrapper(env, wrap_spaces=wrap_spaces) | |||
|
|||
|
|||
def atari_make(id, scalarize=True, **kwargs): | |||
def atari_make(id, scalarize=False, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small docstring?
for more information, see https://pre-commit.ci
PART1.
Add and update code to train on atari games (DQN and PPO on ALE/Breakout and ALE/Freewy):
TODO part 2 (Other PR) : #285